fix: prevent home page crash when latest feed returns 403#63
fix: prevent home page crash when latest feed returns 403#63justinhartman merged 2 commits intodevelopfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e915b7a61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let newMovies = | ||
| movieResult.status === 'fulfilled' ? movieResult.value.data.result || [] : []; | ||
| let newSeries = | ||
| seriesResult.status === 'fulfilled' ? seriesResult.value.data.result || [] : []; |
There was a problem hiding this comment.
When one upstream latest-feed call fails, these lines coerce that dataset to [], and the handler still executes setLatest({ movies: newMovies, series: newSeries }) afterward; because helpers/cache.ts sets a 24-hour TTL, a transient 403/timeout can leave the home page missing movies or series for an entire day even after the API recovers. Consider skipping cache writes (or using a much shorter TTL) when either Promise.allSettled result is rejected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Updated
getHometo avoid writing to the 24-hour latest cache when either upstream latest-feed request fails; it now computeshasFeedFailurefrom thePromise.allSettledresults and only callssetLatestwhen both requests are fulfilled. This preserves resilient rendering while preventing stale partial data from being cached. controllers/appController.tsL85-L113 - Added/updated test coverage to assert
setLatestis not called when one latest-feed request rejects, guarding against regressions for the reviewer-reported scenario. controllers/appController.spec.tsL97-L117 - Committed the change on the current branch with Conventional Commit message:
fix: skip cache write on partial latest-feed failures(commit2ef24f8).
Testing
⚠️ curl -I https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/allSettled | head(network/proxy returned HTTP 403 in this environment).- ✅
yarn test controllers/appController.spec.ts --runInBand
Also completed the required PR handoff step via make_pr with title “fix: avoid caching partial latest-feed fallbacks” and a body summarizing these follow-up changes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #63 +/- ##
===========================================
- Coverage 100.00% 99.62% -0.38%
===========================================
Files 21 21
Lines 1844 1863 +19
Branches 154 163 +9
===========================================
+ Hits 1844 1856 +12
- Misses 0 6 +6
- Partials 0 1 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Motivation
Promise.alland an Axios error propagated.req.paramsbeingstring | string[]to avoid TypeScript and runtime issues.Description
Promise.allSettledand treat each feed independently so a failing feed falls back to an empty array while successful feeds still render.console.warn) when a latest-feed fetch is rejected to aid troubleshooting without interrupting rendering.getViewby coercingreq.params.id,req.params.season, andreq.params.episodeto strings when an array is present to preventstring | string[]type errors.getHome handles upstream failures without crashingthat simulates one rejected feed and verifies the page still renders with partial data.Testing
yarn test controllers/appController.spec.ts --runInBandand the spec file passed (17 passed).yarn lint:tsin this environment but it fails due to missing type definition files forjest/node, which is unrelated to the functional change in this PR.Codex Task